Skip to content

Conversation

@rniwa
Copy link
Contributor

@rniwa rniwa commented Mar 16, 2025

…os_log functions should be treated as safe in call arguments checkers.

Also treat _builtin* functions and __libcpp_verbose_abort functions as "trivial" for the purpose in call argument checkers.

…eated as safe.

os_log functions should be treated as safe in call arguments checkers.

Also treat __builtin_* functions and __libcpp_verbose_abort functions as "trivial"
for the purpose in call argument checkers.
@rniwa rniwa requested review from haoNoQ and t-rasmud March 16, 2025 06:06
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Mar 16, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 16, 2025

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

…os_log functions should be treated as safe in call arguments checkers.

Also treat _builtin* functions and __libcpp_verbose_abort functions as "trivial" for the purpose in call argument checkers.


Full diff: https://github.com/llvm/llvm-project/pull/131500.diff

5 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+9-2)
  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h (+3)
  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp (+3)
  • (modified) clang/test/Analysis/Checkers/WebKit/mock-system-header.h (+1-1)
  • (modified) clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp (+5-1)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index bfa58a11c6199..8724ff3c15acc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -424,6 +424,14 @@ bool isPtrConversion(const FunctionDecl *F) {
   return false;
 }
 
+bool isTrivialBuiltinFunction(const FunctionDecl *F) {
+  if (!F)
+    return false;
+  auto Name = F->getName();
+  return Name.starts_with("__builtin") || Name == "__libcpp_verbose_abort" ||
+         Name.starts_with("os_log") || Name.starts_with("_os_log");
+}
+
 bool isSingleton(const FunctionDecl *F) {
   assert(F);
   // FIXME: check # of params == 1
@@ -601,8 +609,7 @@ class TrivialFunctionAnalysisVisitor
         Name == "isMainThreadOrGCThread" || Name == "isMainRunLoop" ||
         Name == "isWebThread" || Name == "isUIThread" ||
         Name == "mayBeGCThread" || Name == "compilerFenceForCrash" ||
-        Name == "bitwise_cast" || Name.find("__builtin") == 0 ||
-        Name == "__libcpp_verbose_abort")
+        Name == "bitwise_cast" || isTrivialBuiltinFunction(Callee))
       return true;
 
     return IsFunctionTrivial(Callee);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 60bfd1a8dd480..096675fb912f2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -142,6 +142,9 @@ std::optional<bool> isGetterOfSafePtr(const clang::CXXMethodDecl *Method);
 /// pointer types.
 bool isPtrConversion(const FunctionDecl *F);
 
+/// \returns true if \p F is a builtin function which is considered trivial.
+bool isTrivialBuiltinFunction(const FunctionDecl *F);
+
 /// \returns true if \p F is a static singleton function.
 bool isSingleton(const FunctionDecl *F);
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
index d59d03f110776..39e9cd023d1f7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
@@ -246,6 +246,9 @@ class RawPtrRefCallArgsChecker
     if (Callee && TFA.isTrivial(Callee) && !Callee->isVirtualAsWritten())
       return true;
 
+    if (isTrivialBuiltinFunction(Callee))
+      return true;
+
     if (CE->getNumArgs() == 0)
       return false;
 
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-system-header.h b/clang/test/Analysis/Checkers/WebKit/mock-system-header.h
index 73d6e3dbf4643..e993fd697ffab 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-system-header.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-system-header.h
@@ -28,4 +28,4 @@ enum os_log_type_t : uint8_t {
 
 typedef struct os_log_s *os_log_t;
 os_log_t os_log_create(const char *subsystem, const char *category);
-void os_log_msg(os_log_t oslog, os_log_type_t type, const char *msg);
+void os_log_msg(os_log_t oslog, os_log_type_t type, const char *msg, ...);
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
index 0279e2c68ec6d..69842264af56b 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -695,9 +695,13 @@ RefPtr<RefCounted> object();
 void someFunction(const RefCounted&);
 
 void test2() {
-    someFunction(*object());
+  someFunction(*object());
 }
 
 void system_header() {
   callMethod<RefCountable>(object);
 }
+
+void log(RefCountable* obj) {
+  os_log_msg(os_log_create("WebKit", "DOM"), OS_LOG_TYPE_INFO, "obj: %p next: %p", obj, obj->next());
+}
\ No newline at end of file

Copy link
Contributor

@t-rasmud t-rasmud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rniwa
Copy link
Contributor Author

rniwa commented Mar 17, 2025

Thanks for the review!

@rniwa rniwa merged commit 4781941 into llvm:main Mar 18, 2025
9 of 11 checks passed
@rniwa rniwa deleted the treat-os_log-as-safe branch March 18, 2025 06:47
rniwa added a commit to rniwa/llvm-project that referenced this pull request Apr 22, 2025
…eated as safe. (llvm#131500)

…os_log functions should be treated as safe in call arguments checkers.

Also treat __builtin_* functions and __libcpp_verbose_abort functions as
"trivial" for the purpose in call argument checkers.
rniwa added a commit to rniwa/llvm-project that referenced this pull request Aug 21, 2025
…eated as safe. (llvm#131500)

…os_log functions should be treated as safe in call arguments checkers.

Also treat __builtin_* functions and __libcpp_verbose_abort functions as
"trivial" for the purpose in call argument checkers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:static analyzer clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants